-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #1442: add new phase, SelectStatic #1445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
GenBCode has an implicit assumption that I wasn't aware of: GetStatic should not be emitted against a valid selector. If it is, GenBCode messes up the stack by not pop-ing the selector. Surprisingly, this transformation is perfumed in nsc by flatten.
Blocks are not denoting trees(why aren't they?) For now, I'm fixing this using a quick fix. For future, it may make sense to discuss this on dotty meeting and make blocks be a Denoting tree and return denotation of expo. Another option is to move regularisation logic into tree transformers.
The new behaviour is more reasonable. Now the module if forced consistently in both examples. Note that this is deviation from behaviour of scalac.
case Select(Block(stats, qual), nm) => | ||
Block(stats, cpy.Select(t)(qual, nm)) | ||
case Apply(Block(stats, qual), nm) => | ||
Block(stats, Apply(qual, nm)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason not to copy Apply
and TypeApply
as you do with Select
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copying does not copy the tree here, as it always has a different structure.
cpy.Select can preserve the symbol though, in case method is overloaded. This is not needed for Apply\TypeApply as by the time they are applied overloads are already resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I asked because I ran into the overloading issues some time ago. Great!
LGTM, thanks for the quick and elegant fix @DarkDimius! |
A general comment. Try to keep the rest loosely coupled with the backend so they can evolve independently and cages in one area does not necessitate changes elsewhere. |
… objects. This restores parity with the run-time semantics of Scala 2. When accessing an inner static class or object, we avoid loading the enclosing objects as values. The discrepancy in run-time semantics was introduced as a by-product of scala#1445 The change in `t4859.check` restores it to its previous state, before that PR.
… objects. This restores parity with the run-time semantics of Scala 2. When accessing an inner static class or object, we avoid loading the enclosing objects as values. The discrepancy in run-time semantics was introduced as a by-product of scala#1445 The change in `t4859.check` restores it to its previous state, before that PR.
… objects. This restores parity with the run-time semantics of Scala 2. When accessing an inner static class or object, we avoid loading the enclosing objects as values. The discrepancy in run-time semantics was introduced as a by-product of scala#1445 The change in `t4859.check` restores it to its previous state, before that PR.
GenBCode has an implicit assumption that I wasn't aware of:
GetStatic should not be emitted against a non-trivial selector.
If it is, GenBCode messes up the stack by not pop-ing the selector.
Surprisingly, this transformation is perfumed in nsc by flatten.